Parsing the WMA_TAG and using only the release part for creating the WMA_BUILD_ID #1485
Conversation
|
Not to copy paste the test results in multiple places, I am just linking them in the current comment here from the WMcore issue: dmwm/WMCore#11990 (comment) |
There was a problem hiding this comment.
@todor-ivanov changes are looking good to me. However, I have made suggestion for minor improvements.
docker/pypi/wmagent/install.sh
Outdated
| echo | ||
| echo "=======================================================================" | ||
| echo "Starting new WMAgent deployment with the following initialisation data:" | ||
| echo "-----------------------------------------------------------------------" | ||
| echo " - WMAgent Version : $WMA_TAG" | ||
| echo " - WMAgent Tag : $WMA_TAG" |
There was a problem hiding this comment.
I would suggest to keep it saying "WMAgent Version".
Then rephrase "WMAgent Release" to "WMAgent Release Cycle".
Lastly, remove the Major, Minor and Patch, and those might be confusing and maybe even incorrect (e.g., for release 2.3.3).
There was a problem hiding this comment.
ok,
... the way I dd it was to be extra explicit in the meaning and the matching between the print outs and the variables themselves, but that's minor. What I cannot understand is why would the major/minor and patch part of the tag be incorrect for a tag like WMA_TAG=2.3.3 ? I have tested it many times and in the case of a tag like this what those pirntouts would say would be:
#11 0.115 =======================================================================
#11 0.115 Starting new WMAgent deployment with the following initialisation data:
#11 0.115 -----------------------------------------------------------------------
#11 0.116 - WMAgent Tag : 2.3.3
#11 0.116 - WMAgent Release : 2.3.3
#11 0.116 - WMAgent Major version : 2.3
#11 0.116 - WMAgent Minor version : 3
#11 0.116 - WMAgent Patch/Release cand :
#11 0.116 - WMAgent User :
#11 0.116 - WMAgent Root path : /data
#11 0.118 - Python Version : Python 3.8.16
#11 0.118 - Python Module path : /usr/local/lib/python3.8/site-packages
#11 0.118 =======================================================================
Which is exactly what is expected I believe.
There was a problem hiding this comment.
I think the concept of release type for our tagging convention is not straight forward. For instance, for 2.3.4, I would say:
- major release: 2
- middle release: 3
- minor release: 4
but for 2.3.4.5, I see two ways to read it. Either as:
- major release: 2
- middle release: 3
- minor release: 4
- patch release: 5
OR - major release: 2
- middle release: 3.4
- minor release: 5
- patch release:
Due to the potential confusion on this interpretation, I feel like it is better not to give such information.
docker/pypi/wmagent/install.sh
Outdated
| @@ -105,7 +116,8 @@ echo "-----------------------------------------------------------------------" | |||
| stepMsg="Generating and preserving current build id" | |||
| echo "-----------------------------------------------------------------------" | |||
| echo "Start $stepMsg" | |||
| echo $RANDOM |sha256sum |awk '{print $1}' > $WMA_ROOT_DIR/.dockerBuildId | |||
|
|
|||
| echo ${WMA_VER[release]}|sha256sum |awk '{print $1}' > $WMA_ROOT_DIR/.dockerBuildId | |||
There was a problem hiding this comment.
To make it more readable, I would suggest whitespaces around the pipe operation.
docker/pypi/wmagent/install.sh
Outdated
| # Parsing the WMA_TAG | ||
| declare -A WMA_VER | ||
| WMA_VER[release]=$(echo $WMA_TAG|sed -nr 's/.*(^[0-9]+\.[0-9]+\.[0-9]+).*$/\1/p') | ||
| WMA_VER[patch]=$(echo $WMA_TAG|sed -nr 's/.*(^[0-9]+\.[0-9]+\.[0-9]+)//p') |
There was a problem hiding this comment.
To make it more readable, I would suggest whitespaces around the pipe operation. Same for the line above.
todor-ivanov
force-pushed
the
WMAgent_SwitchDeploymentModel_fix-11990
branch
from
7aa7440
to
a0a069f
Compare
…WMA_BUILD_ID Review comments Forgotten space
todor-ivanov
force-pushed
the
WMAgent_SwitchDeploymentModel_fix-11990
branch
from
a0a069f
to
7f27b0e
Compare
|
hi @amaltaro addressed your comments. could you please take another look? |
Fixes: dmwm/WMCore#11990
With the current PR we create the WMA_BUILD_ID based only on the release part of the WMA_TAG of the currently built agent. This way we should avoid restarting initialization process (hence agent data and databse deletion) on patch releases or release candidates.